chore: bump curve25519-dalek from 3.2.1 to 4.1.3 #2252

yihau · 2024-07-23T08:29:07Z

Problem

we did this bump in #1693, but it seems like we have some concerns that broke the semver convention. I revered it in #2055.

Summary of Changes

try to bump it again

samkim-crypto

@yihau Can you take a look at the suggested changes? It seems like we just need to bump-up the opt-level for the curve25519 crate and all the tests should go through (#2683).

.github/scripts/downstream-project-spl-common.sh

Cargo.toml

Co-authored-by: samkim-crypto <[email protected]>

yihau · 2024-08-30T05:00:54Z

thank you @samkim-crypto. it looks good!

mergify · 2024-08-30T05:01:35Z

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

samkim-crypto

Thanks, it looks good to me! Let's just double check with @joncinque and @ilya-bobyr before merging.

joncinque

Looks good to me! Just a couple of comments to clarify things.

It is a bit annoying to have to use the profile override, since downstream devs will need to do the same thing in their workspace, but I can't come up with a better solution.

joncinque · 2024-08-30T18:43:40Z

.github/scripts/downstream-project-spl-common.sh

+echo "[profile.dev.package.curve25519-dalek]
+opt-level = 3" >> Cargo.toml


Rather than overriding this, let's wait for solana-labs/solana-program-library#7220 to land

sure! looks like it has been merged and removed the hack f1d4168 🫡

joncinque · 2024-08-30T18:44:40Z

Cargo.toml

+[profile.dev.package.curve25519-dalek]
+opt-level = 3


Can you give more of an explanation about why this is added? I suggested this language in the SPL PR, but feel free to improve or clarify it:

curve25519-dalek uses the simd backend by default in v4 if possible, which has very slow performance on some platforms with opt-level 0, which is the default for dev and test builds. This slowdown causes certain interactions in the solana-test-validator, such as verifying ZK proofs in transactions, to take much more than 400ms, creating problems in the testing environment.
To enable better performance in solana-test-validator during tests and dev builds, we override the opt-level to 3 for the crate.

added! 7ba52fa

joncinque

Looks great to me, thanks!

* bump curve25519-dalek from 3.2.1 to 4.1.3 * Update .github/scripts/downstream-project-spl-common.sh Co-authored-by: samkim-crypto <[email protected]> * Update .github/scripts/downstream-project-spl-common.sh Co-authored-by: samkim-crypto <[email protected]> * Update .github/scripts/downstream-project-spl-common.sh Co-authored-by: samkim-crypto <[email protected]> * Update Cargo.toml Co-authored-by: samkim-crypto <[email protected]> * remove opt level hack * add comment for opt level --------- Co-authored-by: samkim-crypto <[email protected]> (cherry picked from commit 6e23e69) # Conflicts: # Cargo.lock # Cargo.toml # programs/sbf/Cargo.lock

andrewkmin · 2024-09-03T18:46:58Z

Hey all, thanks for merging this in. Just for the sake of estimation, any idea as to when it might get released? Thanks!

* bump curve25519-dalek from 3.2.1 to 4.1.3 * Update .github/scripts/downstream-project-spl-common.sh Co-authored-by: samkim-crypto <[email protected]> * Update .github/scripts/downstream-project-spl-common.sh Co-authored-by: samkim-crypto <[email protected]> * Update .github/scripts/downstream-project-spl-common.sh Co-authored-by: samkim-crypto <[email protected]> * Update Cargo.toml Co-authored-by: samkim-crypto <[email protected]> * remove opt level hack * add comment for opt level --------- Co-authored-by: samkim-crypto <[email protected]> (cherry picked from commit 6e23e69) # Conflicts: # Cargo.lock # Cargo.toml # programs/sbf/Cargo.lock

Tenari · 2024-09-05T21:35:54Z

ok, but this same issue causes the same problem on the solana-client crate.

Tenari · 2024-09-05T21:38:35Z

for reference what I talking about re: the solana-client dependency:

tenari@dzs-MacBook-Pro api % cargo build
    Updating crates.io index
    Updating git repository `https://github.com/anza-xyz/agave.git`
error: failed to select a version for `zeroize`.
    ... required by package `curve25519-dalek v3.2.1`
    ... which satisfies dependency `curve25519-dalek = "^3.2.1"` of package `solana-program v2.0.0`
    ... which satisfies dependency `solana-program = "^2.0.0"` of package `spl-pod v0.3.0`
    ... which satisfies dependency `spl-pod = "^0.3.0"` of package `spl-token-2022 v4.0.0`
    ... which satisfies dependency `spl-token-2022 = "=4.0.0"` of package `solana-account-decoder v2.1.0 (https://github.com/anza-xyz/agave.git?rev=6e23e69f092fa98cdf172c21e30b244ff1e4bfdf#6e23e69f)`
    ... which satisfies git dependency `solana-account-decoder` of package `solana-pubsub-client v2.1.0 (https://github.com/anza-xyz/agave.git?rev=6e23e69f092fa98cdf172c21e30b244ff1e4bfdf#6e23e69f)`
    ... which satisfies git dependency `solana-pubsub-client` of package `solana-client v2.1.0 (https://github.com/anza-xyz/agave.git?rev=6e23e69f092fa98cdf172c21e30b244ff1e4bfdf#6e23e69f)`
    ... which satisfies git dependency `solana-client` of package `memedeck_api v0.1.0 (/Users/tenari/code/memedeck-monorepo/backend/api)`
versions that meet the requirements `>=1, <1.4` are: 1.3.0, 1.2.0, 1.1.1, 1.1.0, 1.0.0

all possible versions conflict with previously selected packages.

  previously selected package `zeroize v1.7.0`
    ... which satisfies dependency `zeroize = "^1.6"` of package `ruint v1.12.3`
    ... which satisfies dependency `ruint = "^1.12.3"` of package `alloy-primitives v0.7.6`
    ... which satisfies dependency `alloy-primitives = "^0.7.6"` of package `alloy-consensus v0.1.2 (https://github.com/alloy-rs/alloy?tag=v0.1.2#68639726)`
    ... which satisfies git dependency `alloy-consensus` (locked to 0.1.2) of package `alloy v0.1.2 (https://github.com/alloy-rs/alloy?tag=v0.1.2#68639726)`
    ... which satisfies git dependency `alloy` (locked to 0.1.2) of package `generation_api v0.1.0 (/Users/tenari/code/memedeck-monorepo/backend/generation_api)`

failed to select a version for `zeroize` which could resolve this conflict

I get that when I have the following two lines in my Cargo.toml:

solana-sdk = { git = "https://github.com/anza-xyz/agave.git", rev = "6e23e69f092fa98cdf172c21e30b244ff1e4bfdf"  }
solana-client = { git = "https://github.com/anza-xyz/agave.git", rev = "6e23e69f092fa98cdf172c21e30b244ff1e4bfdf"  }

but when I remove the solana-client I no longer get the version error on running cargo build

* bump curve25519-dalek from 3.2.1 to 4.1.3 * Update .github/scripts/downstream-project-spl-common.sh Co-authored-by: samkim-crypto <[email protected]> * Update .github/scripts/downstream-project-spl-common.sh Co-authored-by: samkim-crypto <[email protected]> * Update .github/scripts/downstream-project-spl-common.sh Co-authored-by: samkim-crypto <[email protected]> * Update Cargo.toml Co-authored-by: samkim-crypto <[email protected]> * remove opt level hack * add comment for opt level --------- Co-authored-by: samkim-crypto <[email protected]>

tarcieri mentioned this pull request Aug 7, 2024

Retro compatible for p256 0.10.0 smithy-lang/smithy-rs#3598

Open

steviez mentioned this pull request Aug 22, 2024

Dependency Upgrade - curve25519-dalek #2690

Closed

tarcieri mentioned this pull request Aug 25, 2024

Could you please update zeroize dependency to the newer version? dalek-cryptography/curve25519-dalek#692

Closed

samkim-crypto reviewed Aug 30, 2024

View reviewed changes

.github/scripts/downstream-project-spl-common.sh Outdated Show resolved Hide resolved

.github/scripts/downstream-project-spl-common.sh Outdated Show resolved Hide resolved

.github/scripts/downstream-project-spl-common.sh Show resolved Hide resolved

Cargo.toml Show resolved Hide resolved

bump curve25519-dalek from 3.2.1 to 4.1.3

29e28d4

yihau force-pushed the bump-curve25519 branch from 82024a7 to 29e28d4 Compare August 30, 2024 03:43

yihau and others added 4 commits August 30, 2024 11:47

Update .github/scripts/downstream-project-spl-common.sh

a601fc3

Co-authored-by: samkim-crypto <[email protected]>

Update .github/scripts/downstream-project-spl-common.sh

038e1b9

Co-authored-by: samkim-crypto <[email protected]>

Update .github/scripts/downstream-project-spl-common.sh

bd07e43

Co-authored-by: samkim-crypto <[email protected]>

Update Cargo.toml

6e1edbe

Co-authored-by: samkim-crypto <[email protected]>

yihau marked this pull request as ready for review August 30, 2024 05:00

yihau requested a review from samkim-crypto August 30, 2024 05:01

yihau added the v2.0 Backport to v2.0 branch label Aug 30, 2024

samkim-crypto previously approved these changes Aug 30, 2024

View reviewed changes

samkim-crypto requested review from ilya-bobyr and joncinque August 30, 2024 06:42

samkim-crypto mentioned this pull request Aug 30, 2024

Increase dev mode opt-level for curve25519-dalek crate solana-labs/solana-program-library#7220

Merged

joncinque reviewed Aug 30, 2024

View reviewed changes

remove opt level hack

f1d4168

yihau dismissed samkim-crypto’s stale review via f1d4168 September 1, 2024 06:32

add comment for opt level

7ba52fa

joncinque approved these changes Sep 3, 2024

View reviewed changes

yihau merged commit 6e23e69 into anza-xyz:master Sep 3, 2024
53 checks passed

yihau deleted the bump-curve25519 branch September 3, 2024 09:57

mergify bot mentioned this pull request Sep 3, 2024

v2.0: chore: bump curve25519-dalek from 3.2.1 to 4.1.3 (backport of #2252) #2817

Closed

joncinque mentioned this pull request Sep 3, 2024

ci: Remove all references to twoxtx solana-labs/solana-program-library#7234

Merged

joncinque mentioned this pull request Sep 3, 2024

spl: Bump to newest SPL crates for token-2022 release #2797

Closed

tarcieri mentioned this pull request Sep 4, 2024

curve25519-dalek zeroize dependency not compatible with other libraries solana-labs/solana#26688

Closed

samkim-crypto mentioned this pull request Sep 27, 2024

bump curve25519-dalek from 3.2.1 to 4.1.3 #1693

Merged

tarcieri mentioned this pull request Sep 27, 2024

Update zeroize dependency version please. dalek-cryptography/curve25519-dalek#708

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: bump curve25519-dalek from 3.2.1 to 4.1.3 #2252

chore: bump curve25519-dalek from 3.2.1 to 4.1.3 #2252

yihau commented Jul 23, 2024

samkim-crypto left a comment

yihau commented Aug 30, 2024

mergify bot commented Aug 30, 2024

samkim-crypto left a comment

joncinque left a comment

joncinque Aug 30, 2024

yihau Sep 2, 2024 •

edited

Loading

joncinque Aug 30, 2024

yihau Sep 2, 2024

joncinque left a comment

andrewkmin commented Sep 3, 2024

Tenari commented Sep 5, 2024

Tenari commented Sep 5, 2024

		echo "[profile.dev.package.curve25519-dalek]
		opt-level = 3" >> Cargo.toml

		[profile.dev.package.curve25519-dalek]
		opt-level = 3

chore: bump curve25519-dalek from 3.2.1 to 4.1.3 #2252

chore: bump curve25519-dalek from 3.2.1 to 4.1.3 #2252

Conversation

yihau commented Jul 23, 2024

Problem

Summary of Changes

samkim-crypto left a comment

Choose a reason for hiding this comment

yihau commented Aug 30, 2024

mergify bot commented Aug 30, 2024

samkim-crypto left a comment

Choose a reason for hiding this comment

joncinque left a comment

Choose a reason for hiding this comment

joncinque Aug 30, 2024

Choose a reason for hiding this comment

yihau Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

joncinque Aug 30, 2024

Choose a reason for hiding this comment

yihau Sep 2, 2024

Choose a reason for hiding this comment

joncinque left a comment

Choose a reason for hiding this comment

andrewkmin commented Sep 3, 2024

Tenari commented Sep 5, 2024

Tenari commented Sep 5, 2024

yihau Sep 2, 2024 •

edited

Loading